Skip to content
This repository has been archived by the owner on Aug 12, 2022. It is now read-only.

More Julia v0.4->v0.5 fixes #7188

Merged
merged 3 commits into from
Dec 26, 2016
Merged

Conversation

ma-laforge
Copy link
Contributor

Also try to provide @shashi with a good notebook example, as requested in:

JuliaGizmos/Interact.jl#36

@ma-laforge
Copy link
Contributor Author

@nalimilan
Copy link
Member

FWIW, the changes you made at ma-laforge/LibPSF.jl@7308f14 are not really correct, as they mean you won't accept non-standard string types. Just use Compat.jl without changing the code at all.

Other changes look OK.

@ma-laforge
Copy link
Contributor Author

Thanks for the comment. I might be wrong here (correct me if so), but from what I understood:

File paths (in Julia? / on modern computing systems in general?) are supposed to be stored/manipulated as UTF8 strings (Not UTF16, nor UTF32, nor...).

Now, from my understanding: since v0.5 of Julia, I should be using type String - because UTF8String is now deprecated and merged with ASCIIString.

I used to generate function signatures with ::AbstractString on file paths simply out of laziness. If I used the suggested UTF8String instead, someone making a function call using an ASCIIString would either have to explicitly upconvert to UTF8String - or I (as the module developer) would have to make duplicate function signatures to support ASCIIString file paths (thus the laziness).

As for using String for signal names: I am not convinced the PSF file format even supports UTF8 strings at all. I was debating throwing an exception if !(isascii(signame))... but opted to maintain support for UTF8 strings... just in case it worked.

@kmsquire
Copy link
Member

Bump. I think that the discussion is ancillary, and that this can be merged--is this correct?

@ma-laforge
Copy link
Contributor Author

@kmsquire: If the question was directed at me, than yes: This PR is ready/waiting for a merge.

@ViralBShah ViralBShah merged commit 7925813 into JuliaLang:metadata-v2 Dec 26, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants